Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for artists and albumartists multi-valued tags #4743

Merged
merged 43 commits into from
Sep 9, 2023

Conversation

jmbannon
Copy link
Contributor

@jmbannon jmbannon commented Apr 5, 2023

Description

#505

Piggy-backs on the work done in #4582 to add support for:

multi-tags as part of the default behavior when fetching tags from musicbrainz. The ones marked accordingly require mediafile >=0.12.0 to write the tags to the file. setup.py is updated to reflect this.

I think albumtypes has paved the way on how multi tags should be handled - using a delimiter and storing them in the database as a string, but writing them to tags as a multi (list).

Some considerations with this approach are:

  • It requires no massive rewrite/migration/etc, falls in line with the 'KISS' database schema that beets has adopted
  • Searching multi-fields will require regex (i.e. artists::Eric Clapton) since the db's literal value will be Eric Clapton\␀B.B. King
  • mediafile abstracts whether it's actually written as a multi, or with a separator when using id3v23
    • Right now it uses / for a delimiter, example above would write Eric Clapton/B.B. King

Potential follow-ups:

To Do

  • Documentation. (If you've add a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
  • Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
  • Tests. (Encouraged but not strictly required.)

self.tracks = tracks
self.asin = asin
self.albumtype = albumtype
self.albumtypes = albumtypes or []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was missing from the other PR

@arsaboo
Copy link
Contributor

arsaboo commented Apr 6, 2023

Oh my God... this would be amazing. Can you add basic docs for someone to test this out? I'll test this out and report findings.

@jmbannon
Copy link
Contributor Author

jmbannon commented Apr 6, 2023

Oh my God... this would be amazing. Can you add basic docs for someone to test this out? I'll test this out and report findings.

I haven't tested this yet aside from unit tests. I too will give it a spin when I get more free time.

@jmbannon
Copy link
Contributor Author

jmbannon commented Apr 6, 2023

I imported this album on my branch and verified that albumartists are indeed writing multi tags 🎉

As for testing docs @arsaboo , multi tags for artists/albumartists should work out of the box - no additional configuration needed.

I'll add some more unit tests to make sure id3v23 works as expected

artist = recording['artist-credit'][0]['artist']
info.artist_id = artist['id']
info.artists, info.artists_sort, info.artists_credits = \
_multi_artist_credit(recording['artist-credit'], include_join_phrase=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth rewriting _multi_artist_credit and _flatten_credit_artist to only scan the artist-credit once. Currently, it's doing two passes

@jmbannon
Copy link
Contributor Author

Any thoughts on this @sampsyo ? Would love some feedback on this - and sorry for being impatient 😅

Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow!!! This is incredibly great work—many commendations, @jmbannon. I am super impressed by how simple these changes ended up being, especially given how low the bug number is for #505. 😃 I'm sure we could find an even more ambitious, complex way to approach this, but this seems like a very reasonable way to get most of the benefit without changing everything all at once.

I had a couple of very tiny comments within, but if this seems to work for everybody and passes CI, I think it's basically good to go. Again, huge thanks to @jmbannon for tackling a seemingly-intractable problem with aplomb.

@@ -74,15 +74,19 @@ def __init__(
album_id: Optional[str] = None,
artist: Optional[str] = None,
artist_id: Optional[str] = None,
artists: Optional[List[str]] = None,
artists_ids: Optional[List[str]] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, we've been gradually moving away from listing each field exhaustively in AlbumInfo/TrackInfo and instead just relying on **kwargs to convey all the information we need. But no big deal if it's easier to grok with these fields listed explicitly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As boiler-plate as it is, I prefer it lol. For me, it helps to know which native field types are expected

)


def _flatten_artist_credit(credit: List[Dict]) -> Tuple[str, str, str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work on this refactoring. Clever and also simple!

@jmbannon
Copy link
Contributor Author

jmbannon commented Apr 18, 2023

I am super impressed by how simple these changes ended up being, especially given how low the bug number is for #505

I too was humbled by how simple this was. Goes to show I'm standing on the shoulders of giants

One test failed because python <=3.8 and/or sqlite doesn't like the literal null terminator as a DB value (would only return the first element). Changed it to \␀. For future work like adding a contains search syntax for multi's, this will be the delimiter used to create the underlying regex. Mediafile won't actually use this separator since the values get passed in as a list.

@JOJ0
Copy link
Member

JOJ0 commented Sep 1, 2023

@JOJ0 I disagree with you, I think many people would LOVE the feature @jmbannon has added, bugs or not. He did what nobody could do in over a decade, its amazing.

@sampsyo, @JOJ0 are you going to implement @jmbannon suggest on Github? I know you complained about talking about Github, but he just showed you how to make it better instead of wasting everyones time and turning away great developers who want to contribute.

I appreciate this lesson and now very much understand why forks happen in this community, its a shame we can't all work together.

Dear @catduckgnaf, I want to be honest with you: Two days ago I was quite motivated to move on with this PR and help jmbannon move forward. But after our debate and in the end receiving the message I quoted above, I'm not at all motivated to move on with that topic. You are accusing me of several things: I'm wasting your time, I'm the reason that people walk away from the project and you are telling me "how it should be done".

Do you really think that helps anyone?

Well, we might have very different opinions on how Open Source projects should be handled, and certainly your opinions are valuable, BUT that does not mean you should throw them at us in that manner. Please go ahead and "politely" submit your suggestions into a fitting thread like that one: #4558

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 1, 2023

@JOJ0 I appreciate the time you volunteer to push beets forward. I'll apologize on behalf of @catduckgnaf , the bottom-line is we're both eager to see this PR through and it's been a bit frustrating waiting for a few months. But it's important to remember that open source doesn't owe us anything, including fast reviews/approvals/etc from folks like you who spend time maintaining great software for free.

I'm still determined to do what needs to be done to get this feature pushed so everyone can enjoy its value. I hope we can continue collaborating - I would love to expand multi-tag functionality beyond this PR. That requires good terms and healthy collaboration, which I hope we can all commit to going forward.

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 1, 2023

I too will try better going forward, even if it means going against my own personal best practices. Tomorrow I'll get this PR force pushed as a single commit like you asked

@JOJ0
Copy link
Member

JOJ0 commented Sep 1, 2023

I too will try better going forward, even if it means going against my own personal best practices. Tomorrow I'll get this PR force pushed as a single commit like you asked

No. Please dont rebase to one single commit. I never said I want a single commit. Please read my initial message again: #4743 (comment)

Too summarize, I was talking about:

  • not loosing history all over!
  • reordering commits if it makes sense
  • useful chunks that make sense

And now if you were really to tidy it up, I'd like to add:

  • good commit messages, not single words, that tell, even to someone who is knew to that parts of the codebase what is happening here (but I think I also mentioned that in that message)

But I have to admit that things changed in other parts of my life during the last days also and I have to finish something by the end of September, so probably October will be a better time to move on with "in detail / new territorry" beets things.

So again, and also as written in that linked first comment, my offer was that, I could at least try to test the "usage" of the feature as a first step, to get a feeling from a user's perspective. What I currently can't offer, is reviewing the code in detail. Too much new territorry, I'm not the person for that. That's all I can offer but I'm interested how it works on the long run, but it will take time which I currently dont have.

@JOJ0
Copy link
Member

JOJ0 commented Sep 1, 2023

@JOJ0 I appreciate the time you volunteer to push beets forward. I'll apologize on behalf of @catduckgnaf , the bottom-line is we're both eager to see this PR through and it's been a bit frustrating waiting for a few months. But it's important to remember that open source doesn't owe us anything, including fast reviews/approvals/etc from folks like you who spend time maintaining great software for free.

I'm still determined to do what needs to be done to get this feature pushed so everyone can enjoy its value. I hope we can continue collaborating - I would love to expand multi-tag functionality beyond this PR. That requires good terms and healthy collaboration, which I hope we can all commit to going forward.

Thanks for the appreciation and the kind words and that you think about those things. Yes let's get back to healthy corporation, I'm sure we can do it :-)

@catduckgnaf
Copy link

@JOJ0 I am sorry and wanted to apologize.
I simply want to see this great software become even better!

@JOJ0
Copy link
Member

JOJ0 commented Sep 3, 2023

@JOJ0 I am sorry and wanted to apologize.

I simply want to see this great software become even better!

Apology accepted. :-) We will do that! Soonish!

sentriz added a commit to sentriz/betanin that referenced this pull request Sep 7, 2023
@JOJ0
Copy link
Member

JOJ0 commented Sep 8, 2023

I tried my best to go ahead and wrap my head around this feature. I read the comments history twice, the media file additions PR and did - at least - some playing around with it myself.

Thanks for testing over there @arsaboo, that helped a lot to make me understand some things without having to look at code or play around myself initially. Thanks to you both @arsaboo and @jmbannon, just reading your conversation helped already!!

Some playing around with a "two-artists-release"

Database:

$ beet info -l fracture binga drift | grep -e artists -e ids
        albumartists: Fracture\␀Sam Binga
 albumartists_credit: Fracture\␀Sam Binga
   albumartists_sort: Fracture\␀Binga, Sam
             artists: Fracture\␀Sam Binga
      artists_credit: Fracture\␀Sam Binga
         artists_ids:
        artists_sort: Fracture\␀Binga, Sam
   mb_albumartistids: 96999054-38f8-47da-8906-a6b763aa37ce\␀bfc15bf3-2904-4d2c-990b-5e2c350654b0
        mb_artistids: 96999054-38f8-47da-8906-a6b763aa37ce\␀bfc15bf3-2904-4d2c-990b-5e2c350654b

File tags via beets, prior to updating mediafile (still 0.11.0):

$ beet info fracture binga drift | grep -e artists -e ids
      albumartists: Fracture; Sam Binga
           artists: Fracture; Sam Binga
 mb_albumartistids: 96999054-38f8-47da-8906-a6b763aa37ce; bfc15bf3-2904-4d2c-990b-5e2c350654b0
      mb_artistids: 96999054-38f8-47da-8906-a6b763aa37ce; bfc15bf3-2904-4d2c-990b-5e2c350654b0

File tags via mutagen-inspect, prior to updating mediafile (still 0.11.0):

$ mutagen-inspect "/home/jojo/Documents/Sync/beets_test_music/Uncategorized/Fracture & Sam Binga - 2022 - Omura/10 - Fracture & Sam Binga - Omura - 6 Foot Drift (2022).mp3" | grep -i -e artists -e ids
TXXX=ALBUMARTISTS=Fracture / Sam Binga
TXXX=ALBUMARTISTSORT=Fracture & Binga, Sam
TXXX=ALBUM_ARTISTS=Fracture / Sam Binga
TXXX=ARTISTS=Fracture / Sam Binga

File tags via mutagen-inspect, prior to updating mediafile (still 0.11.0):

$ mutagen-inspect "/home/jojo/Documents/Sync/beets_test_music/Uncategorized/Fracture & Sam Binga - 2022 - Omura/10 - Fracture & Sam Binga - Omura - 6 Foot Drift (2022).mp3" | grep -i -e artists -e ids
TXXX=ALBUMARTISTS=Fracture / Sam Binga
TXXX=ALBUMARTISTSORT=Fracture & Binga, Sam
TXXX=ALBUM_ARTISTS=Fracture / Sam Binga
TXXX=ARTISTS=Fracture / Sam Binga

Updated mediafile to 0.12, writing tags:

$ beet write fracture binga drift
Fracture & Sam Binga - Omura - 10 - 6 Foot Drift (2022/XW)  [APHALP004] f:MP3-128kCBR a:70-01/ASIS |Electro, Metal, Drum And Bass
  artists_sort:  -> Fracture\␀Binga, Sam
  artists_credit:  -> Fracture\␀Sam Binga
  albumartists_sort:  -> Fracture\␀Binga, Sam
  albumartists_credit:  -> Fracture\␀Sam Binga

File tags beets now sees:

$ beet info fracture binga drift | grep -e artists -e ids
       albumartists: Fracture; Sam Binga
albumartists_credit: Fracture; Sam Binga
  albumartists_sort: Fracture; Binga, Sam
            artists: Fracture; Sam Binga
     artists_credit: Fracture; Sam Binga
       artists_sort: Fracture; Binga, Sam
  mb_albumartistids: 96999054-38f8-47da-8906-a6b763aa37ce; bfc15bf3-2904-4d2c-990b-5e2c350654b0
       mb_artistids: 96999054-38f8-47da-8906-a6b763aa37ce; bfc15bf3-2904-4d2c-990b-5e2c350654b0

File tags mutagen-inspect now sees:

$ mutagen-inspect "/home/jojo/Documents/Sync/beets_test_music/Uncategorized/Fracture & Sam Binga - 2022 - Omura/10 - Fracture & Sam Binga - Omura - 6 Foot Drift (2022).mp3" | grep -i -e artists -e ids
TXXX=ALBUMARTISTS=Fracture / Sam Binga
TXXX=ALBUMARTISTSORT=Fracture & Binga, Sam
TXXX=ALBUMARTISTS_CREDIT=Fracture / Sam Binga
TXXX=ALBUMARTISTS_SORT=Fracture / Binga, Sam
TXXX=ALBUM_ARTISTS=Fracture / Sam Binga
TXXX=ARTISTS=Fracture / Sam Binga
TXXX=ARTISTS_CREDIT=Fracture / Sam Binga
TXXX=ARTISTS_SORT=Fracture / Binga, Sam

First of all, is what I'm seeing here the expected outcome?

@JOJ0
Copy link
Member

JOJ0 commented Sep 8, 2023

I'm very close to wanting to hit that merge button. Some questions come up:

  • Where (in the code) is the replacement of the delimiter we save in the database with the one we finally save in the files tags happening? In this case it seems like a semicolon is used when saving (;).

  • In your initial description of this PR you are suggesting some possible next steps. Which route would you like to go?

  • Does the thumbs-up you put on my last message mean "Yes I'd like to take a minute and rebase some commits". In any case, I have another proposal what could be done to make that history survive (which I would prefer) but make it more valueable, with just a little work: Use git rebase -i <commit_before_first_feature_commit> and then instead of using reordering and squashing, you could simply use the "reword" feature and rewrite some of those "single word" commit messages to short sentences. It would keep all your commits as-is, but help "future code reviewers" a lot when they stumble across those changes in the master branch eg. a couple of years. We will never know what might happen to github/gitlab/or wherever the future hosts code - but what will always remain with us because it's undesdructible - is the actual git history! It would be wonderful if single-word-commits could be just reworded.

Some random examples from somewhere around beets, how a descriptive commit messages could look like:

    replaygain: better types to represent peak methods

    thanks @redacted_user in review https://github.com/beetbox/beets/pull/3996 for
    this suggestion to properly represent the sum type
    replaygain test: extend tests for skipping items

    Test more combinations of tags that might initially be present and
    expected tags. The R128 codepath and the case of having the wrong type
    of tags wasn't really tested before.

I know that not everyone is as tidy / likes to write a lot of words as the authors above do - but at least quite a lot of our contributors do -
We don't need exhaustive sentences here and not even more than a headline, but when I look through our git history it just helps so tremendously if I at least see "part of beets / plugin name: tiny headline what's it about"

I'm trying again to make this happen - if not - I'll give up ;-)

  • I see we have very little documentation on how to use this, but maybe it's too early to exhaustivle describe this feature. What's your plans?

  • How is the decisions going around how this can be used from the cli without regex. Did you do a final decision on how this will function. I need to probably read this all again since I'm not entirely following this yet: Multi-value query syntax discussion #4792 (reply in thread)

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 8, 2023

@JOJ0 Much thanks for all your time testing this out. Over the course of today, I'll try to respond to all of your questions from top-to-bottom

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 8, 2023

@JOJ0 All your commands look good. In the database, it writes Fracture\␀Binga, Sam to indicate usage of the id3v2.4 multi-tag separator.

I'm not familiar with mutagen-inspect - I assume / means it's separate with id3v2.4 null separator under-the-hood, especially since beets reports with ; as the 'human-readable' separator.

Where (in the code) is the replacement of the delimiter we save in the database with the one we finally save in the files tags happening? In this case it seems like a semicolon is used when saving (;).

See

class DelimitedString(BaseString[List[str], List[str]]):
- it's a class that orchestrates list-based field representation. When we write to:

  • mediafile: the value itself will be in list form (passed within beets/autotag/__init__.py). The unit tests show the expected lists within the mediafile object, which indicates it supports id3v2.4 tagging.
  • database: Will write using the to_sql method in the DelimitedString class, which does a join using the separator. This is seen in your update command
  • beets info: It will see if the value is a list, and print them using .join("; "). This happens for any list-based field including albumtypes (which created the DelimitedString helper class). I forget where this happens, but I remember seeing it because I too was concerned it wasn't writing properly

In your initial description of this PR you are suggesting some possible next steps. Which route would you like to go?

The ones on the top-of my mind are:

  • more explicit printing within info indicating tags are multi
  • better multi-search (as seen in the discussion linked in the description)

I suggest we do these in a follow-up PR since this one is already pretty big

I have another proposal what could be done to make that history survive

I really think we should squash merge this as a single-commit to master since it's adding a single feature with tests to beets. My concern regarding history was just for the PR/branch. Future reviewers can refer to this PR to get the history. And worst-case if we need to revert the commit, it will be seamless since it's packaged into a single one.

I see we have very little documentation on how to use this, but maybe it's too early to exhaustivle describe this feature. What's your plans?

IMO I don't think this PR needs any additional docs added to the readthedocs - it's simply adding more musicbrainz fields (which aren't documented as-is), and there already is a notion of using id3v2.3 vs id3v2.4 tags here: https://beets.readthedocs.io/en/stable/reference/config.html?highlight=id3#id3v23

How is the decisions going around how this can be used from the cli without regex.

Nothing is in stone yet, but @sampsyo provided some good wisdom on how we should approach it. Again, I think it's okay to put this off for a future PR + reboot the discussion. Any multi-tag, regardless of id3v2.3 vs 2.4 has the same issue - it's not specific to this PR changeset.

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 8, 2023

how a descriptive commit messages could look like:

GitHub lets you change the commit message when you squash it. If you're okay with that approach, I can draft a message that you can use if/when you do the squash + merge

@JOJ0
Copy link
Member

JOJ0 commented Sep 8, 2023

how a descriptive commit messages could look like:

GitHub lets you change the commit message when you squash it. If you're okay with that approach, I can draft a message that you can use if/when you do the squash + merge

Alright, then lets go that way and I'll hit that merge+squash button :-)

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 8, 2023

@JOJ0 Sounds good! I'll get something drafted later today

@jmbannon
Copy link
Contributor Author

jmbannon commented Sep 8, 2023

Draft message for the squash merge commit, feel free to change as you see fit:

multi-tags: add support for `artists`, `albumartists` related multi-tags (#4743)

  Adds the following fields with id3v2.4 multi-tag support to autotag:
    - artists
    - artists_sort
    - artists_credit
    - albumartists
    - albumartists_sort
    - albumartists_credit
    - mb_artistids
    - mb_albumartistids
  MusicBrainz support to populate + write the above multi-tags by default. Can be toggled to use id3v2.3 or id3v2.4 tags via existing beets configuration.
  Big thanks to @JOJ0, @OxygenCobalt, @arsaboo for testing + @sampsyo for the initial code review 

and fix wording to use "multi-valued tags" instead of "multi-tags".
@JOJ0 JOJ0 changed the title Add support for artists and albumartists multi tags Add support for artists and albumartists multi-valued tags Sep 9, 2023
@JOJ0 JOJ0 merged commit f72261e into beetbox:master Sep 9, 2023
14 checks passed
@JOJ0
Copy link
Member

JOJ0 commented Sep 9, 2023

@jmbannon I did some nitpicking on the wording in docs, changelog and PR/issue naming. Sometimes we referred as multi tags, multi-tags, multi-valued tags, multiple-valued tags, and I decided that mutli-valued tags is the most descriptive one which still is not too longish. Also in code we have MULTI_VALUE_DSV for example which fits with that wording. Trying to make sure wording is easy to grasp for readers not "in to" the topic yet. HTH :-)

I was thinking about adding an example about what you wrote in the docs "use regex" but didn't in the end since I trust that you will do that soon in your next PR, and maybe we have better options for searching then already :-)

Looking forward to your next contributions, thanks so much for this awesome feature, sorry that it all took so long and speak soon :-)

@jmbannon jmbannon deleted the j/multi branch September 9, 2023 14:50
Louson pushed a commit to Louson/beets that referenced this pull request Sep 26, 2023
)

Adds the following fields with id3v2.4 multi-valued tag support to autotag:
    - artists, artists_sort, artists_credit
    - albumartists, albumartists_sort, albumartists_credit
    - mb_artistids, mb_albumartistids

MusicBrainz support to populate + write the above multi-valued tags by default. Can be toggled to use id3v2.3 or id3v2.4 tags via the existing beets configuration option `id3v23`.

Big thanks to @JOJ0, @OxygenCobalt, @arsaboo for testing + @sampsyo for the initial code review .
@KingDuckZ
Copy link

KingDuckZ commented Mar 22, 2024

I've been searching around for a way to split the new arrays, this is not working for me: default: '%first{$albumartists}' by the looks of it it's naming directories with the internal representation of the arrays, so with the nullchar. How can I get just the first entry?

Edit: just to report, this works for me: %first{$albumartists,1,0,\␀} however is this the intended behaviour?

@JOJ0
Copy link
Member

JOJ0 commented Mar 22, 2024

maybe @jmbannon can clarify what's the intended way of doing it

@KingDuckZ
Copy link

While waiting I'm trying to import my music and noticed another issue with the rewrite plugin, it crashes beets if I try to alter albumartists with it. Looking at the backtrace, it tries to convert a list into lowercase. Just my guess but this change intoduced lists where previously there could only be strings?

@snejus
Copy link
Member

snejus commented May 17, 2024

I'm using current master (c75f07a) where I made a single change in discogs.py:
image

I'm trying to provide a list of artist_ids to AlbumInfo on album import.

However, I keep receiving this issue

  File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1865, in main
    _raw_main(args)
  File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1852, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/sarunas/repo/beets/beets/ui/commands.py", line 1395, in import_func
    import_files(lib, paths, query)
  File "/home/sarunas/repo/beets/beets/ui/commands.py", line 1326, in import_files
    session.run()
  File "/home/sarunas/repo/beets/beets/importer.py", line 362, in run
    pl.run_sequential()
  File "/home/sarunas/repo/beets/beets/util/pipeline.py", line 391, in run_sequential
    list(self.pull())
  File "/home/sarunas/repo/beets/beets/util/pipeline.py", line 469, in pull
    next_msgs.extend(_allmsgs(out))
  File "/home/sarunas/repo/beets/beets/util/pipeline.py", line 468, in pull
    out = coro.send(msg)
  File "/home/sarunas/repo/beets/beets/util/pipeline.py", line 171, in coro
    task = func(*(args + (task,)))
  File "/home/sarunas/repo/beets/beets/importer.py", line 1568, in user_query
    apply_choice(session, task)
  File "/home/sarunas/repo/beets/beets/importer.py", line 1641, in apply_choice
    task.add(session.lib)
  File "/home/sarunas/repo/beets/beets/importer.py", line 818, in add
    self.album = lib.add_album(self.imported_items())
  File "/home/sarunas/repo/beets/beets/library.py", line 1610, in add_album
    item.add(self)
  File "/home/sarunas/repo/beets/beets/library.py", line 403, in add
    super().add(lib)
  File "/home/sarunas/repo/beets/beets/dbcore/db.py", line 654, in add
    self.store()
  File "/home/sarunas/repo/beets/beets/library.py", line 395, in store
    super().store(fields)
  File "/home/sarunas/repo/beets/beets/dbcore/db.py", line 591, in store
    tx.mutate(
  File "/home/sarunas/repo/beets/beets/dbcore/db.py", line 962, in mutate
    cursor = self.db._connection().execute(statement, subvals)
sqlite3.InterfaceError: Error binding parameter 2 - probably unsupported type.

And worst of all - if this happens when I try to reimport an album that already exists in the database (beet import -LI), after this error it's gone from the database, together with all flexible attributes, including play_count etc. which cannot be recovered.

@jmbannon have you encountered this by any chance? Or am I missing something in my setup?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants